feat: initial libp2p integration with req-resp status decoding#7
Merged
MegaRedHand merged 7 commits intomainfrom Jan 6, 2026
Merged
feat: initial libp2p integration with req-resp status decoding#7MegaRedHand merged 7 commits intomainfrom
MegaRedHand merged 7 commits intomainfrom
Conversation
pablodeymo
added a commit
that referenced
this pull request
Mar 13, 2026
…ttestation processing Three defensive fixes in process_attestations / try_finalize: 1. aggregation_bits OOB (finding #6): A malicious attestation could carry aggregation_bits longer than the validator set, causing votes[validator_id] to panic on out-of-bounds access. Now filter bits >= validator_count before indexing. 2. root_to_slot missing key (finding #7): try_finalize used direct HashMap index root_to_slot[root] which panics if a justification root is absent from historical_block_hashes (e.g. carried over from a prior finalization window). Replaced with .get() — missing roots are conservatively retained. 3. justifications[root] (finding #8): Reviewed and confirmed safe — the roots come from justifications.keys() so the key is always present. No change needed.
pablodeymo
added a commit
that referenced
this pull request
Mar 16, 2026
…on data (#228) ## Motivation Security audit findings #6, #7, #8: three sites in `process_attestations` / `try_finalize` use direct indexing on data structures whose keys or indices come from network-controlled attestations. A malicious peer can craft attestations that trigger panics, crashing the node. ## Description ### Fix 1 — Reject attestations with OOB `aggregation_bits` (finding #6) **File:** `crates/blockchain/state_transition/src/lib.rs` **Problem:** `aggregation_bits` is a `BitList<ValidatorRegistryLimit>` (max 4096 bits) deserialized from the network. `votes` is a `Vec<bool>` of length `validator_count` (actual validators in state). If a malicious attestation carries `aggregation_bits` with bits set beyond `validator_count`, direct indexing panics. **Spec and cross-client behavior:** | Implementation | Behavior | |---|---| | **Spec** (state.py:503-505) | Direct list access `justifications[target.root][validator_id]` — crashes on OOB (IndexError) | | **Zeam** (Zig) | Rejects attestation with error | | **Lantern** (C) | Rejects attestation with `return -1` | | **gean** (Go) | `Bitlist.Get()` returns false for OOB (silent skip) | **Fix:** Pre-loop length check that rejects the entire attestation when `aggregation_bits.len() > validator_count`, matching the spec (crash = implicit reject) and the majority of other clients (Zeam, Lantern). Direct indexing `votes[validator_id] = true` is safe after the bounds check. --- ### Fix 2 — Prune justification roots missing from `root_to_slot` (finding #7) **File:** `crates/blockchain/state_transition/src/lib.rs` (in `try_finalize`) **Problem:** `root_to_slot` is built from `historical_block_hashes` for slots after finalization. But `justifications` can contain roots carried over from a previous finalization window that no longer appear in `historical_block_hashes`. Direct HashMap index panics on missing key. **Spec and cross-client behavior:** | Implementation | Behavior | |---|---| | **Spec** (state.py:560-562) | `assert all(root in root_to_slot for root in justifications)` — crashes (invariant violation) | | **Lantern** (C) | Conservative retention | The spec treats missing roots as an invariant violation. A root absent from `root_to_slot` means its slot is at or below `finalized_slot` (already finalized). Conservatively retaining such roots (previous approach with `is_none_or`) would cause them to accumulate forever — they never reappear in `root_to_slot`, and `is_valid_vote` rejects new votes for them. **Fix:** Prune missing roots (return `false` in `retain`), matching the spec's filter intent: `root_to_slot[root] > finalized_slot` would be `false` for such roots. A warning is logged when this happens. --- ### Finding #8 — `justifications[root]` (no change needed) **File:** `crates/blockchain/state_transition/src/lib.rs` (in `serialize_justifications`) ```rust let justification_roots: Vec<H256> = justifications.keys().cloned().collect(); // ... .flat_map(|root| justifications[root].iter()) ``` Reviewed and confirmed safe: `justification_roots` is built from `justifications.keys()`, so every root used as index is guaranteed to be present in the map. No change needed. ## How to Test ```bash make fmt make lint cargo test --workspace --release ``` All 120 tests pass unchanged — valid attestations always have `aggregation_bits.len() <= validator_count` and all justification roots are in `root_to_slot`, so these guards only trigger on malformed/adversarial data not present in test fixtures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds logic for dialing to peers, and decoding and printing incoming Status messages.